Skip to content

Implement Result::getQueryResult() - #14

Open
simon-mundy wants to merge 2 commits into
php-db:0.1.xfrom
simon-mundy:result-get-query-result
Open

Implement Result::getQueryResult()#14
simon-mundy wants to merge 2 commits into
php-db:0.1.xfrom
simon-mundy:result-get-query-result

Conversation

@simon-mundy

Copy link
Copy Markdown
Member

Summary

Fixes #12. Driver\ResultInterface::getQueryResult() arrived in php-db/phpdb#172 and Pgsql\Result never implemented it, so loading the class was a fatal and composer update followed by composer test failed before a single test ran.

Implemented as Pdo\Result does upstream: guard on isQueryResult(), clone the given prototype or a default ResultSet, initialize it from this result.

php-db/phpdb 0.6.x-dev requires php ^8.3, so the lock cannot pick up the interface change while the package still allows 8.2. The php constraint, config.platform.php and the PhpStan check in .laminas-ci.json move to 8.3 with it. That is part of #10, brought forward here because the fix cannot install otherwise.

Test plan

test/unit/ResultTest.php covers the seeded result set, the prototype clone and the rejection, using PhpDbTestAsset\Pgsql\ResultStub to answer isQueryResult() and getFieldCount() — the only two methods getQueryResult() reaches that read the pgsql resource, which needs a live connection.

test/integration/ResultTest.php covers the real path through the native driver: a select seeding a result set with the right field count and rows, and SET search_path TO public as a statement with no fields.

phpcs, phpstan and the unit suite pass. The integration suite has not been run locally — no Postgres available here — so it runs for the first time in CI.

Notes

Ordering: this should land before #11, which is blocked on it. Six of that PR's matrix legs fail with this fatal, and its codecov and mutation-test jobs stay skipped while any test leg is red. #11 then rebases and drops its own copy of the php constraint change.

composer audit reports a high-severity command injection advisory against squizlabs/php_codesniffer (CVE-2026-67434, fixed in 3.13.6). It arrives through laminas/laminas-coding-standard, which #11 removes, so no fix here.

php-db/phpdb#172 added getQueryResult() to Driver\ResultInterface, so
Pgsql\Result no longer satisfied the contract and loading the class was a
fatal before any test could run. Follows Pdo\Result upstream: guard on
isQueryResult(), clone the given prototype or a default ResultSet, and
initialize it from this result.

php-db/phpdb 0.6.x-dev requires php ^8.3, so the php constraint,
config.platform.php and the PhpStan check in .laminas-ci.json move to 8.3 —
the lock cannot pick up the interface change otherwise.

Covered by unit tests over a ResultStub that answers the two methods
reading the pgsql resource, and by integration tests running a real select
and a fields-less statement through the native driver.
The PhpStan check in .laminas-ci.json runs on latest dependencies, so it
picks up phpstan 2.2, which no longer reports the two implode
argument.type errors in Metadata\Source. The baseline still ignored them,
so the job failed on ignore.unmatched, which is non-ignorable.

2.1 does still report them, so the entries cannot simply be removed while
the lock stays on 2.1; require-dev moves to ^2.2 so both the locked and
latest jobs analyse with a version that agrees. Pre-existing on 0.1.x, and
unrelated to getQueryResult, but it fails this PR's CI.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Result does not implement ResultInterface::getQueryResult() — fatal against current php-db/phpdb 0.6.x-dev

1 participant